Skip to content

Raise error for invalid smiles and return None #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
aditya0by0 wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Raise error for invalid smiles and return None #138

aditya0by0 wants to merge 5 commits into from

Conversation

@aditya0by0
Copy link
Member

@aditya0by0 aditya0by0 commented Dec 15, 2025 •
edited
Loading

Fixes #137

@aditya0by0 aditya0by0 linked an issue Dec 15, 2025 that may be closed by this pull request
ChemDataReader.read_data incorrectly accepts entire invalid strings with atleast one valid token #137
Open
@aditya0by0 aditya0by0 self-assigned this Dec 15, 2025
@aditya0by0
Copy link
Member Author

aditya0by0 commented Dec 15, 2025 •
edited
Loading

RDKIT behavior changed in latest version

raw_data = CC(=O)NC1CC1[Mg-2]

                mol = Chem.MolFromSmiles(raw_data.strip())
                if mol is not None:
                    raw_data = Chem.MolToSmiles(mol, canonical=True)

rdkit 2024.3.6 returns CC(=O)NC1CC1[Mg-2] (version on my local machine)
rdkit 2025.9.3 returns CC(=O)NC1C[CH]1[Mg-2] (latest version)

This is the reason causing failure of the test.

@sfluegel05
Do we need to pin the rdkit version (as per #83) or should we look for an upgrade ?

@sfluegel05 sfluegel05 mentioned this pull request Dec 16, 2025
Open
@aditya0by0
Copy link
Member Author

aditya0by0 commented Dec 17, 2025

@sfluegel05, this PR is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfluegel05 sfluegel05 Awaiting requested review from sfluegel05

Assignees

@aditya0by0 aditya0by0

Labels

bug:fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ChemDataReader.read_data incorrectly accepts entire invalid strings with atleast one valid token

2 participants

@aditya0by0